Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/testscript: allow -e flag to pass env value #122

Merged
merged 1 commit into from
Jan 19, 2021
Merged

Conversation

myitcv
Copy link
Collaborator

@myitcv myitcv commented Jan 19, 2021

The -e flag for testscript currently allows the caller to specify that
environment variables are inherited from the current environment.

However this falls when the current environment does not have a value
set and you need to set a specific value. For example:

GOBUILD=$(go env GOBUILD) testscript -e GOBUILD script.txt

Instead, allow -e to also specify a value:

testscript -e GOBUILD=$(go env GOBUILD) script.txt

which feels more fluent/natural.

@myitcv myitcv requested a review from rogpeppe January 19, 2021 06:34
@myitcv myitcv force-pushed the allow_env_var_values branch 2 times, most recently from ce7213b to b38072b Compare January 19, 2021 09:15
Copy link
Owner

@rogpeppe rogpeppe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with a couple of minor suggestions, thanks!

cmd/testscript/main.go Outdated Show resolved Hide resolved
cmd/testscript/main.go Show resolved Hide resolved
cmd/testscript/main.go Show resolved Hide resolved
The -e flag for testscript currently allows the caller to specify that
environment variables are inherited from the current environment.

However this falls when the current environment does not have a value
set and you need to set a specific value. For example:

    GOBUILD=$(go env GOBUILD) testscript -e GOBUILD script.txt

Instead, allow -e to also specify a value:

    testscript -e GOBUILD=$(go env GOBUILD) script.txt

which feels more fluent/natural.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants